Use eachById in mailbox:clean
#137
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
"Implicitly marking parameter $container as nullable is deprecated, the explicit nullable type must be used instead"
Fix for a deprecated parameter:
|
Perfect, thank you! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I added
mailbox:cleanto an application that has been using this package for several years, and noticed that older emails weren't being cleared out unless I ran the command multiple times.Reading the command, I noticed it used
chunkwhen deleting records. The problem withchunkin this context is that it usesoffset/limitpagination under the hood. As rows are deleted in each chunk, the remaining rows shift, causing the next chunk to skip some records. This results in incomplete deletions unless the command is repeatedly executed.To verify the issue, I modified
CleanEmailsTestto create 200 records instead of 60, which caused the test to fail due to records being skipped.To fix this, the command now uses
eachById, which paginates based on primary key rather than offset. This avoids skipping rows during deletion, since it always continues from the last seen ID, not a shifting offset.